-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[plugin] Fixed wrong type conversion #6351
Conversation
I do not think an implicit conversion is the right thing to do, though. But this at least fixes the current state. In a cleaner approach we should make the protocol work with shapes solely actually using and implementing the protocol from here https://github.com/microsoft/vscode/blob/master/src/vs/workbench/api/common/extHost.protocol.ts#L369 See also #6353 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -435,7 +436,9 @@ namespace ObjectsTransferrer { | |||
// tslint:disable-next-line:no-any | |||
const obj: any = JSON.parse(value.data); | |||
// May require to use types-impl there instead of vscode lang server Range for the revival | |||
return Range.create(Position.create(obj.start.line, obj.start.character), Position.create(obj.end.line, obj.end.character)); | |||
const start = new Position(obj.start.line, obj.start.character); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
☝️ this was already foreseen, kind of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see one line above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, git it. I'll remove the comment
The type (un)marshalling was wrongly using language server types instead of vscode.d.ts / theia.d.ts types. Signed-off-by: Sven Efftinge <sven.efftinge@typefox.io>
a782bde
to
108ec6a
Compare
Actually, defining the parameters using interface types opens us up to people passing things that don't serialize properly (the "Position" class form types-impl.ts, for example.
|
What it does
The type (un)marshalling was wrongly using
language server types instead of vscode.d.ts /
theia.d.ts types.
How to test
It revealed in the gitlens extensions, which uses document symbols from other language services in order to position code lenses. The resulting ranges and positions were send back to our document implementation which failed because of an unsatisfied
instanceof Position
check.I.e. install the gitlens extensions (together with native VS Code git) and see an error in the console and no code lenses on the symbols (it produces a fall back code lens on top of the file, though).
Review checklist
Reminder for reviewers